goInstallTools: don't lint with staticcheck when it's enabled in gopls
If a user has staticcheck enabled via gopls, we shouldn't require them
to install the staticcheck binary separately. Also, we do not run the
goLint function when the lintTool is staticcheck and staticcheck is
enabled in gopls.
Change-Id: I2b6c1260bdf1e5de0cb1d0009b25a752c434cd31
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/301053
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index 47d151d..cd7652d 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -13,7 +13,7 @@
import path = require('path');
import semver = require('semver');
import { ConfigurationTarget } from 'vscode';
-import { getGoConfig } from './config';
+import { getGoConfig, getGoplsConfig } from './config';
import { toolExecutionEnvironment, toolInstallationEnvironment } from './goEnv';
import { addGoRuntimeBaseToPATH, clearGoRuntimeBaseFromPATH } from './goEnvironmentStatus';
import { logVerbose } from './goLogging';
@@ -51,7 +51,7 @@
export async function installAllTools(updateExistingToolsOnly = false) {
const goVersion = await getGoVersion();
- let allTools = getConfiguredTools(goVersion, getGoConfig());
+ let allTools = getConfiguredTools(goVersion, getGoConfig(), getGoplsConfig());
// exclude tools replaced by alternateTools.
const alternateTools: { [key: string]: string } = getGoConfig().get('alternateTools');
@@ -551,7 +551,7 @@
}
function getMissingTools(goVersion: GoVersion): Promise<Tool[]> {
- const keys = getConfiguredTools(goVersion, getGoConfig());
+ const keys = getConfiguredTools(goVersion, getGoConfig(), getGoplsConfig());
return Promise.all<Tool>(
keys.map(
(tool) =>
diff --git a/src/goLint.ts b/src/goLint.ts
index 685c644..4bf6138 100644
--- a/src/goLint.ts
+++ b/src/goLint.ts
@@ -5,10 +5,11 @@
import path = require('path');
import vscode = require('vscode');
-import { getGoConfig } from './config';
+import { getGoConfig, getGoplsConfig } from './config';
import { toolExecutionEnvironment } from './goEnv';
import { lintDiagnosticCollection } from './goMain';
import { diagnosticsStatusBarItem, outputChannel } from './goStatus';
+import { goplsStaticcheckEnabled } from './goTools';
import { getWorkspaceFolderPath, handleDiagnosticErrors, ICheckResult, resolvePath, runTool } from './util';
/**
* Runs linter on the current file, package or workspace.
@@ -28,12 +29,13 @@
const documentUri = editor ? editor.document.uri : null;
const goConfig = getGoConfig(documentUri);
+ const goplsConfig = getGoplsConfig(documentUri);
outputChannel.clear(); // Ensures stale output from lint on save is cleared
diagnosticsStatusBarItem.show();
diagnosticsStatusBarItem.text = 'Linting...';
- goLint(documentUri, goConfig, scope)
+ goLint(documentUri, goConfig, goplsConfig, scope)
.then((warnings) => {
handleDiagnosticErrors(editor ? editor.document : null, warnings, lintDiagnosticCollection, 'go-lint');
diagnosticsStatusBarItem.hide();
@@ -54,8 +56,14 @@
export function goLint(
fileUri: vscode.Uri,
goConfig: vscode.WorkspaceConfiguration,
+ goplsConfig: vscode.WorkspaceConfiguration,
scope?: string
): Promise<ICheckResult[]> {
+ const lintTool = goConfig['lintTool'] || 'staticcheck';
+ if (lintTool === 'staticcheck' && goplsStaticcheckEnabled(goConfig, goplsConfig)) {
+ return;
+ }
+
epoch++;
const closureEpoch = epoch;
if (tokenSource) {
@@ -74,7 +82,6 @@
return Promise.resolve([]);
}
- const lintTool = goConfig['lintTool'] || 'staticcheck';
const lintFlags: string[] = goConfig['lintFlags'] || [];
const lintEnv = toolExecutionEnvironment();
const args: string[] = [];
diff --git a/src/goMain.ts b/src/goMain.ts
index 5da1a9d..3cf4872 100644
--- a/src/goMain.ts
+++ b/src/goMain.ts
@@ -9,7 +9,7 @@
'use strict';
import * as path from 'path';
-import { getGoConfig, initConfig, IsInCloudIDE } from './config';
+import { getGoConfig, getGoplsConfig, initConfig, IsInCloudIDE } from './config';
import { browsePackages } from './goBrowsePackage';
import { buildCode } from './goBuild';
import { check, notifyIfGeneratedFile, removeTestStatus } from './goCheck';
@@ -925,7 +925,7 @@
outputChannel.appendLine('');
const goVersion = await getGoVersion();
- const allTools = getConfiguredTools(goVersion, getGoConfig());
+ const allTools = getConfiguredTools(goVersion, getGoConfig(), getGoplsConfig());
allTools.forEach((tool) => {
const toolPath = getBinPath(tool.name);
diff --git a/src/goTools.ts b/src/goTools.ts
index 6839349..8e9dbe2 100644
--- a/src/goTools.ts
+++ b/src/goTools.ts
@@ -109,7 +109,11 @@
return tool.name === 'gocode' || tool.name === 'gocode-gomod';
}
-export function getConfiguredTools(goVersion: GoVersion, goConfig: { [key: string]: any }): Tool[] {
+export function getConfiguredTools(
+ goVersion: GoVersion,
+ goConfig: { [key: string]: any },
+ goplsConfig: { [key: string]: any }
+): Tool[] {
// If language server is enabled, don't suggest tools that are replaced by gopls.
// TODO(github.com/golang/vscode-go/issues/388): decide what to do when
// the go version is no longer supported by gopls while the legacy tools are
@@ -172,8 +176,12 @@
maybeAddTool(getFormatTool(goConfig));
}
- // Add the linter that was chosen by the user.
- maybeAddTool(goConfig['lintTool']);
+ // Add the linter that was chosen by the user, but don't add staticcheck
+ // if it is enabled via gopls.
+ const goplsStaticheckEnabled = useLanguageServer && goplsStaticcheckEnabled(goConfig, goplsConfig);
+ if (goConfig['lintTool'] !== 'staticcheck' || !goplsStaticheckEnabled) {
+ maybeAddTool(goConfig['lintTool']);
+ }
// Add the language server if the user has chosen to do so.
// Even though we arranged this to run after the first attempt to start gopls
@@ -189,6 +197,17 @@
return tools;
}
+export function goplsStaticcheckEnabled(
+ goConfig: { [key: string]: any },
+ goplsConfig: { [key: string]: any }
+): boolean {
+ if (goConfig['useLanguageServer'] !== true || goplsConfig['ui.diagnostic.staticcheck'] !== true) {
+ return false;
+ }
+ const features = goConfig['languageServerExperimentalFeatures'];
+ return !features || features['diagnostics'] === true;
+}
+
export const allToolsInformation: { [key: string]: Tool } = {
'gocode': {
name: 'gocode',
diff --git a/test/integration/extension.test.ts b/test/integration/extension.test.ts
index cd69f9d..bd8dc3d 100644
--- a/test/integration/extension.test.ts
+++ b/test/integration/extension.test.ts
@@ -12,7 +12,7 @@
import * as path from 'path';
import * as sinon from 'sinon';
import * as vscode from 'vscode';
-import { getGoConfig } from '../../src/config';
+import { getGoConfig, getGoplsConfig } from '../../src/config';
import { FilePatch, getEdits, getEditsFromUnifiedDiffStr } from '../../src/diffUtils';
import { check } from '../../src/goCheck';
import { GoDefinitionProvider } from '../../src/goDeclaration';
@@ -399,10 +399,11 @@
lintTool: { value: process.platform !== 'win32' ? 'sleep' : 'timeout' },
lintFlags: { value: process.platform !== 'win32' ? ['2'] : ['/t', '2'] }
});
+ const goplsConfig = Object.create(getGoplsConfig(), {});
const results = await Promise.all([
- goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_1.go')), config),
- goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_2.go')), config)
+ goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_1.go')), config, goplsConfig),
+ goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_2.go')), config, goplsConfig)
]);
assert.equal(util.runTool.callCount, 2, 'should have launched 2 lint jobs');
assert.equal(
@@ -431,6 +432,7 @@
// but this test depends on ST1003 (MixedCaps package name) presented in both files
// in the same package. So, enable that.
}),
+ Object.create(getGoplsConfig(), {}),
'package'
);
diff --git a/test/integration/install.test.ts b/test/integration/install.test.ts
index d966000..22ec768 100644
--- a/test/integration/install.test.ts
+++ b/test/integration/install.test.ts
@@ -209,21 +209,21 @@
suite('getConfiguredTools', () => {
test('do not require legacy tools when using language server', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: true });
+ const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: true }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(got.includes('gopls'), `omitted 'gopls': ${JSON.stringify(got)}`);
assert(!got.includes('guru') && !got.includes('gocode'), `suggested legacy tools: ${JSON.stringify(got)}`);
});
test('do not require gopls when not using language server', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: false });
+ const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: false }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(!got.includes('gopls'), `suggested 'gopls': ${JSON.stringify(got)}`);
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
});
test('do not require gopls when the go version is old', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.9'), { useLanguageServer: true });
+ const configured = getConfiguredTools(fakeGoVersion('1.9'), { useLanguageServer: true }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(!got.includes('gopls'), `suggested 'gopls' for old go: ${JSON.stringify(got)}`);
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
