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 #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>

src/goInstallTools.ts

@@ -48,7 +48,7 @@ const declinedInstalls: Tool[] = [];
 
 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 @@ export async function installTool(
  // 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 @@ export async function offerToInstallTools() {
  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 @@ export async function offerToInstallTools() {
 }
 
 function getMissingTools(goVersion: GoVersion): Promise<Tool[]> {
- const keys = getConfiguredTools(goVersion);
+ const keys = getConfiguredTools(goVersion, getGoConfig());
  return Promise.all<Tool>(
  keys.map(
  (tool) =>

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 @@ export function activate(ctx: vscode.ExtensionContext) {
  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 @@ function configureLanguageServer(ctx: vscode.ExtensionContext) {
  };
 
  // Start the language server, or fallback to the default language providers.
- startLanguageServerWithFallback(ctx, true);
-
+ return startLanguageServerWithFallback(ctx, true);
 }
 
 function getCurrentGoPathCommand() {
@@ -753,7 +750,7 @@ async function getConfiguredGoToolsCommand() {
  outputChannel.appendLine('');
 
  const goVersion = await getGoVersion();
- const allTools = getConfiguredTools(goVersion);
+ const allTools = getConfiguredTools(goVersion, getGoConfig());
 
  allTools.forEach((tool) => {
  const toolPath = getBinPath(tool.name);

src/goTools.ts

@@ -18,6 +18,7 @@ export interface Tool {
  name: string;
  importPath: string;
  isImportant: boolean;
+ replacedByGopls?: boolean;
  description: string;
 
  // latestVersion and latestVersionTimestamp are hardcoded default values
@@ -107,12 +108,20 @@ export function isGocode(tool: Tool): boolean {
  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 @@ export function getConfiguredTools(goVersion: GoVersion): Tool[] {
  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 @@ export function getConfiguredTools(goVersion: GoVersion): Tool[] {
  // 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 @@ export const allToolsInformation: { [key: string]: Tool } = {
  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 @@ export const allToolsInformation: { [key: string]: Tool } = {
  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 @@ export const allToolsInformation: { [key: string]: Tool } = {
  '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 @@ export const allToolsInformation: { [key: string]: Tool } = {
  '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'
  }

test/integration/install.test.ts

@@ -15,8 +15,8 @@ import util = require('util');
 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 buildFakeProxy(tools: string[]) {
 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`);
+}