[vscode-go/release] [release] goInstallTools: don't lint with staticcheck when it's enabled in gopls
194 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Mar 11, 2021, 9:31:46 PM3/11/21
to goph...@pubsubhelper.golang.org, Rebecca Stambler, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Hyang-Ah Hana Kim has uploaded this change for review.
[release] 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 <rsta...@golang.org>
Run-TryBot: Rebecca Stambler <rsta...@golang.org>
TryBot-Result: kokoro <noreply...@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
(cherry picked from commit 0058bd16ba31394f98aa3396056998e4808998a7)
---
M src/goInstallTools.ts
M src/goLint.ts
M src/goMain.ts
M src/goTools.ts
M test/integration/extension.test.ts
M test/integration/install.test.ts
6 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index f5e5b58..29010cb 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -13,7 +13,7 @@
import path = require('path');
import { SemVer } from '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');
@@ -540,7 +540,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 45ceb32..027bce5 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';
@@ -924,7 +924,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 b0bf7b3..d978813 100644
--- a/src/goTools.ts
+++ b/src/goTools.ts
@@ -95,7 +95,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
@@ -155,8 +159,12 @@
// Add the format tool that was chosen by the user.
maybeAddTool(goConfig['formatTool']);
- // 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
@@ -172,6 +180,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 e5bf64d..681a1d6 100644
--- a/test/integration/install.test.ts
+++ b/test/integration/install.test.ts
@@ -175,21 +175,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)}`);
To view, visit change 301150. To unsubscribe, or for help writing mail filters, visit settings.
Rebecca Stambler (Gerrit)
Mar 12, 2021, 12:43:54 PM3/12/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim.
Patch set 1:Code-Review +2
Hyang-Ah Hana Kim (Gerrit)
Apr 19, 2021, 9:17:06 PM4/19/21
to Rebecca Stambler, Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Hyang-Ah Hana Kim abandoned this change.
Abandoned
merged in v0.24.0
To view, visit change 301150. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages