src/goInstallTools: stop requiring to install legacy tools
...if the language server is enabled.
Furthermore, mark dlv and gopls as important tools, so
'offerToInstallTools' don't drop them from missing tools list.
And minor bug fix in choosing the go binary used for tool installation.
For tools installation, we are currently trying to find the go binary
from the current GOROOT. In practice, the extension computes the current
GOROOT early on during its activation. But that may fail, and in tests
that skips activation, we can't make the same assumption. So, add a
fallback - if the current goroot isn't set yet, use the goVersion's bin
path.
Corner case:
It's possible that the language server is enabled from the configuration
but the extension fails to start the language server for some reason,
and falls back to the legacy mode. Since I don't know exactly when
this getConfiguredTools will get invoked (before language server started,
after it started or failed, or some other random moment by users'
action), I feel less confident about using the current language
server's runtime status. A better solution would be - before falling
back to the legacy mode silently - we should notify users of this
fallback with the explanation, so users address the underlying issues
that prevented the language server start.
Fixes golang/vscode-go#51
Change-Id: I9d5bf5b8ff59678d8c4ae8f6206e50bb40315329
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/276556
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index cb23c3a..e395ec6 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -48,7 +48,7 @@
export async function installAllTools(updateExistingToolsOnly: boolean = false) {
const goVersion = await getGoVersion();
- let allTools = getConfiguredTools(goVersion);
+ let allTools = getConfiguredTools(goVersion, getGoConfig());
// exclude tools replaced by alternateTools.
const alternateTools: { [key: string]: string } = getGoConfig().get('alternateTools');
@@ -204,7 +204,9 @@
// the current directory path. In order to avoid choosing a different go,
// we will explicitly use `GOROOT/bin/go` instead of goVersion.binaryPath
// (which can be a wrapper script that switches 'go').
- const goBinary = path.join(getCurrentGoRoot(), 'bin', correctBinname('go'));
+ const goBinary = getCurrentGoRoot() ?
+ path.join(getCurrentGoRoot(), 'bin', correctBinname('go')) :
+ goVersion.binaryPath;
// Build the arguments list for the tool installation.
const args = ['get', '-v'];
@@ -449,8 +451,9 @@
title: 'Show',
command() {
outputChannel.clear();
+ outputChannel.show();
outputChannel.appendLine('Below tools are needed for the basic features of the Go extension.');
- missing.forEach((x) => outputChannel.appendLine(x.name));
+ missing.forEach((x) => outputChannel.appendLine(` ${x.name}`));
}
};
vscode.window
@@ -495,7 +498,7 @@
}
function getMissingTools(goVersion: GoVersion): Promise<Tool[]> {
- const keys = getConfiguredTools(goVersion);
+ const keys = getConfiguredTools(goVersion, getGoConfig());
return Promise.all<Tool>(
keys.map(
(tool) =>
diff --git a/src/goMain.ts b/src/goMain.ts
index bba7a75..278be4d 100644
--- a/src/goMain.ts
+++ b/src/goMain.ts
@@ -7,9 +7,7 @@
'use strict';
import * as path from 'path';
-import { commands } from 'vscode';
import vscode = require('vscode');
-import { extensionId } from './const';
import { browsePackages } from './goBrowsePackage';
import { buildCode } from './goBuild';
import { check, notifyIfGeneratedFile, removeTestStatus } from './goCheck';
@@ -111,15 +109,15 @@
suggestUpdates(ctx);
offerToInstallLatestGoVersion();
offerToInstallTools();
- configureLanguageServer(ctx);
+ await configureLanguageServer(ctx);
if (
+ !languageServerIsRunning &&
vscode.window.activeTextEditor &&
vscode.window.activeTextEditor.document.languageId === 'go' &&
isGoPathSet()
) {
// Check mod status so that cache is updated and then run build/lint/vet
- // TODO(hyangah): skip if the language server is used (it will run build too)
isModSupported(vscode.window.activeTextEditor.document.uri).then(() => {
runBuilds(vscode.window.activeTextEditor.document, getGoConfig());
});
@@ -711,8 +709,7 @@
};
// Start the language server, or fallback to the default language providers.
- startLanguageServerWithFallback(ctx, true);
-
+ return startLanguageServerWithFallback(ctx, true);
}
function getCurrentGoPathCommand() {
@@ -753,7 +750,7 @@
outputChannel.appendLine('');
const goVersion = await getGoVersion();
- const allTools = getConfiguredTools(goVersion);
+ const allTools = getConfiguredTools(goVersion, getGoConfig());
allTools.forEach((tool) => {
const toolPath = getBinPath(tool.name);
diff --git a/src/goTools.ts b/src/goTools.ts
index 9aad76c..e9387ba 100644
--- a/src/goTools.ts
+++ b/src/goTools.ts
@@ -18,6 +18,7 @@
name: string;
importPath: string;
isImportant: boolean;
+ replacedByGopls?: boolean;
description: string;
// latestVersion and latestVersionTimestamp are hardcoded default values
@@ -107,12 +108,20 @@
return tool.name === 'gocode' || tool.name === 'gocode-gomod';
}
-export function getConfiguredTools(goVersion: GoVersion): Tool[] {
+export function getConfiguredTools(goVersion: GoVersion, goConfig: { [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
+ // no longer working (or we remove the legacy language feature providers completely).
+ const useLanguageServer = goConfig['useLanguageServer'] && goVersion.gt('1.11');
+
const tools: Tool[] = [];
function maybeAddTool(name: string) {
const tool = allToolsInformation[name];
if (tool) {
- tools.push(tool);
+ if (!useLanguageServer || !tool.replacedByGopls) {
+ tools.push(tool);
+ }
}
}
@@ -146,8 +155,6 @@
maybeAddTool('gocode-gomod');
}
- const goConfig = getGoConfig();
-
// Add the doc/def tool that was chosen by the user.
switch (goConfig['docsTool']) {
case 'godoc':
@@ -164,8 +171,10 @@
// Add the linter that was chosen by the user.
maybeAddTool(goConfig['lintTool']);
- // Add the language server for Go versions > 1.10 if user has choosen to do so.
- if (goConfig['useLanguageServer'] && goVersion.gt('1.10')) {
+ // 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
+ // this is still useful if we've fail to start gopls.
+ if (useLanguageServer) {
maybeAddTool('gopls');
}
@@ -181,6 +190,7 @@
name: 'gocode',
importPath: 'github.com/mdempsky/gocode',
isImportant: true,
+ replacedByGopls: true,
description: 'Auto-completion, does not work with modules',
close: async (env: NodeJS.Dict<string>): Promise<string> => {
const toolBinPath = getBinPath('gocode');
@@ -204,128 +214,150 @@
name: 'gocode-gomod',
importPath: 'github.com/stamblerre/gocode',
isImportant: true,
+ replacedByGopls: true,
description: 'Auto-completion, works with modules',
minimumGoVersion: semver.coerce('1.11'),
},
'gopkgs': {
name: 'gopkgs',
importPath: 'github.com/uudashr/gopkgs/v2/cmd/gopkgs',
+ replacedByGopls: false, // TODO(github.com/golang/vscode-go/issues/258): disable Add Import command.
isImportant: true,
description: 'Auto-completion of unimported packages & Add Import feature'
},
'go-outline': {
name: 'go-outline',
importPath: 'github.com/ramya-rao-a/go-outline',
+ replacedByGopls: false, // TODO(github.com/golang/vscode-go/issues/1020): replace with Gopls.
isImportant: true,
- description: 'Go to symbol in file'
+ description: 'Go to symbol in file' // GoDocumentSymbolProvider, used by 'run test' codelens
},
'go-symbols': {
name: 'go-symbols',
importPath: 'github.com/acroca/go-symbols',
+ replacedByGopls: true,
isImportant: false,
description: 'Go to symbol in workspace'
},
'guru': {
name: 'guru',
importPath: 'golang.org/x/tools/cmd/guru',
+ replacedByGopls: true,
isImportant: false,
description: 'Find all references and Go to implementation of symbols'
},
'gorename': {
name: 'gorename',
importPath: 'golang.org/x/tools/cmd/gorename',
+ replacedByGopls: true,
isImportant: false,
description: 'Rename symbols'
},
'gomodifytags': {
name: 'gomodifytags',
importPath: 'github.com/fatih/gomodifytags',
+ replacedByGopls: false,
isImportant: false,
description: 'Modify tags on structs'
},
'goplay': {
name: 'goplay',
importPath: 'github.com/haya14busa/goplay/cmd/goplay',
+ replacedByGopls: false,
isImportant: false,
description: 'The Go playground'
},
'impl': {
name: 'impl',
importPath: 'github.com/josharian/impl',
+ replacedByGopls: false,
isImportant: false,
description: 'Stubs for interfaces'
},
'gotype-live': {
name: 'gotype-live',
importPath: 'github.com/tylerb/gotype-live',
+ replacedByGopls: true, // TODO(github.com/golang/vscode-go/issues/1021): recommend users to turn off.
isImportant: false,
description: 'Show errors as you type'
},
'godef': {
name: 'godef',
importPath: 'github.com/rogpeppe/godef',
+ replacedByGopls: true,
isImportant: true,
description: 'Go to definition'
},
'gogetdoc': {
name: 'gogetdoc',
importPath: 'github.com/zmb3/gogetdoc',
+ replacedByGopls: true,
isImportant: true,
description: 'Go to definition & text shown on hover'
},
'gofumports': {
name: 'gofumports',
importPath: 'mvdan.cc/gofumpt/gofumports',
+ replacedByGopls: true,
isImportant: false,
description: 'Formatter'
},
'gofumpt': {
name: 'gofumpt',
importPath: 'mvdan.cc/gofumpt',
+ replacedByGopls: true,
isImportant: false,
description: 'Formatter'
},
'goimports': {
name: 'goimports',
importPath: 'golang.org/x/tools/cmd/goimports',
+ replacedByGopls: true,
isImportant: true,
description: 'Formatter'
},
'goreturns': {
name: 'goreturns',
importPath: 'github.com/sqs/goreturns',
+ replacedByGopls: true,
isImportant: true,
description: 'Formatter'
},
'goformat': {
name: 'goformat',
importPath: 'winterdrache.de/goformat/goformat',
+ replacedByGopls: true,
isImportant: false,
description: 'Formatter'
},
- 'golint': {
- name: 'golint',
- importPath: 'golang.org/x/lint/golint',
- isImportant: true,
- description: 'Linter',
- minimumGoVersion: semver.coerce('1.9'),
- },
'gotests': {
name: 'gotests',
importPath: 'github.com/cweill/gotests/...',
+ replacedByGopls: false,
isImportant: false,
description: 'Generate unit tests',
minimumGoVersion: semver.coerce('1.9'),
},
+ // TODO(github.com/golang/vscode-go/issues/189): consider disabling lint when gopls is turned on.
+ 'golint': {
+ name: 'golint',
+ importPath: 'golang.org/x/lint/golint',
+ replacedByGopls: false,
+ isImportant: true,
+ description: 'Linter',
+ minimumGoVersion: semver.coerce('1.9'),
+ },
'staticcheck': {
name: 'staticcheck',
importPath: 'honnef.co/go/tools/...',
+ replacedByGopls: false,
isImportant: true,
description: 'Linter'
},
'golangci-lint': {
name: 'golangci-lint',
importPath: 'github.com/golangci/golangci-lint/cmd/golangci-lint',
+ replacedByGopls: false,
isImportant: true,
description: 'Linter'
},
@@ -338,7 +370,8 @@
'gopls': {
name: 'gopls',
importPath: 'golang.org/x/tools/gopls',
- isImportant: false,
+ replacedByGopls: false, // lol
+ isImportant: true,
description: 'Language Server from Google',
minimumGoVersion: semver.coerce('1.12'),
latestVersion: semver.coerce('0.5.1'),
@@ -349,18 +382,21 @@
'dlv': {
name: 'dlv',
importPath: 'github.com/go-delve/delve/cmd/dlv',
- isImportant: false,
+ replacedByGopls: false,
+ isImportant: true,
description: 'Debugging'
},
'fillstruct': {
name: 'fillstruct',
importPath: 'github.com/davidrjenni/reftools/cmd/fillstruct',
+ replacedByGopls: true,
isImportant: false,
description: 'Fill structs with defaults'
},
'godoctor': {
name: 'godoctor',
importPath: 'github.com/godoctor/godoctor',
+ replacedByGopls: true,
isImportant: false,
description: 'Extract to functions and variables'
}
diff --git a/test/integration/install.test.ts b/test/integration/install.test.ts
index 9a3732e..d73350d 100644
--- a/test/integration/install.test.ts
+++ b/test/integration/install.test.ts
@@ -15,8 +15,8 @@
import vscode = require('vscode');
import { toolInstallationEnvironment } from '../../src/goEnv';
import { installTools } from '../../src/goInstallTools';
-import { allToolsInformation, getTool, getToolAtVersion } from '../../src/goTools';
-import { getBinPath, getGoVersion, rmdirRecursive } from '../../src/util';
+import { allToolsInformation, getConfiguredTools, getTool, getToolAtVersion } from '../../src/goTools';
+import { getBinPath, getGoVersion, GoVersion, rmdirRecursive } from '../../src/util';
import { correctBinname } from '../../src/utils/pathUtils';
suite('Installation Tests', function () {
@@ -166,3 +166,30 @@
function shouldRunSlowTests(): boolean {
return !!process.env['VSCODEGO_BEFORE_RELEASE_TESTS'];
}
+
+suite('getConfiguredTools', () => {
+ test('do not require legacy tools when using language server', async () => {
+ 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 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 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)}`);
+ });
+});
+
+function fakeGoVersion(version: string) {
+ return new GoVersion('/path/to/go', `go version go${version} windows/amd64`);
+}
