Skip to content

Commit 16eadfa

Browse files
committed
src/goMain: suggest tool updates if tools were compiled with old go
Suggest update only if the go major version is newer than the go version used to compile the tools. And change GoVersion constructor to reject the old dev version format used before go1.17. Fixes #1998 Change-Id: I9aeea0438d5530b1b0f6190784a9eb489efbb877 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/385181 Trust: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Suzy Mueller <[email protected]> Trust: Suzy Mueller <[email protected]>
1 parent 934911a commit 16eadfa

File tree

5 files changed

+186
-19
lines changed

5 files changed

+186
-19
lines changed

src/goInstallTools.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -703,9 +703,15 @@ async function defaultInspectGoToolVersion(
703703
path golang.org/x/tools/gopls
704704
mod golang.org/x/tools/gopls (devel)
705705
dep github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
706+
707+
if the binary was built with a dev version of go, in module mode.
708+
/Users/hakim/go/bin/gopls: devel go1.18-41f485b9a7 Mon Jan 31 13:43:52 2022 +0000
709+
path golang.org/x/tools/gopls
710+
mod golang.org/x/tools/gopls v0.8.0-pre.1 h1:6iHi9bCJ8XndQtBEFFG/DX+eTJrf2lKFv4GI3zLeDOo=
711+
...
706712
*/
707713
const lines = stdout.split('\n', 3);
708-
const goVersion = lines[0].split(/\s+/)[1];
714+
const goVersion = lines[0] && lines[0].match(/\s+(go\d+.\d+\S*)/)[1];
709715
const moduleVersion = lines[2].split(/\s+/)[3];
710716
return { goVersion, moduleVersion };
711717
} catch (e) {

src/goMain.ts

+83-4
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ import {
7272
testPrevious,
7373
testWorkspace
7474
} from './goTest';
75-
import { getConfiguredTools } from './goTools';
75+
import { getConfiguredTools, Tool } from './goTools';
7676
import { vetCode } from './goVet';
7777
import { pickGoProcess, pickProcess } from './pickProcess';
7878
import {
@@ -95,6 +95,7 @@ import {
9595
getGoVersion,
9696
getToolsGopath,
9797
getWorkspaceFolderPath,
98+
GoVersion,
9899
handleDiagnosticErrors,
99100
isGoPathSet,
100101
resolvePath
@@ -828,10 +829,88 @@ function checkToolExists(tool: string) {
828829
}
829830
}
830831

832+
// exported for testing
833+
export async function listOutdatedTools(configuredGoVersion: GoVersion, allTools: Tool[]): Promise<Tool[]> {
834+
if (!configuredGoVersion || !configuredGoVersion.sv) {
835+
return [];
836+
}
837+
838+
const { major, minor } = configuredGoVersion.sv;
839+
840+
const oldTools = await Promise.all(
841+
allTools.map(async (tool) => {
842+
const toolPath = getBinPath(tool.name);
843+
if (!path.isAbsolute(toolPath)) {
844+
return;
845+
}
846+
const m = await inspectGoToolVersion(toolPath);
847+
if (!m) {
848+
console.log(`failed to get go tool version: ${toolPath}`);
849+
return;
850+
}
851+
const { goVersion } = m;
852+
if (!goVersion) {
853+
// TODO: we cannot tell whether the tool was compiled with a newer version of go
854+
// or compiled in an unconventional way.
855+
return;
856+
}
857+
const toolGoVersion = new GoVersion('', `go version ${goVersion} os/arch`);
858+
if (!toolGoVersion || !toolGoVersion.sv) {
859+
return tool;
860+
}
861+
if (
862+
major > toolGoVersion.sv.major ||
863+
(major === toolGoVersion.sv.major && minor > toolGoVersion.sv.minor)
864+
) {
865+
return tool;
866+
}
867+
// special case: if the tool was compiled with beta or rc, and the current
868+
// go version is a stable version, let's ask to recompile.
869+
if (
870+
major === toolGoVersion.sv.major &&
871+
minor === toolGoVersion.sv.minor &&
872+
(goVersion.includes('beta') || goVersion.includes('rc')) &&
873+
// We assume tools compiled with different rc/beta need to be recompiled.
874+
// We test the inequality by checking whether the exact beta or rc version
875+
// appears in the `go version` output. e.g.,
876+
// configuredGoVersion.version goVersion(tool) update
877+
// 'go version go1.18 ...' 'go1.18beta1' Yes
878+
// 'go version go1.18beta1 ...' 'go1.18beta1' No
879+
// 'go version go1.18beta2 ...' 'go1.18beta1' Yes
880+
// 'go version go1.18rc1 ...' 'go1.18beta1' Yes
881+
// 'go version go1.18rc1 ...' 'go1.18' No
882+
// 'go version devel go1.18-deadbeaf ...' 'go1.18beta1' No (* rare)
883+
!configuredGoVersion.version.includes(goVersion)
884+
) {
885+
return tool;
886+
}
887+
return;
888+
})
889+
);
890+
return oldTools.filter((tool) => !!tool);
891+
}
892+
831893
async function suggestUpdates(ctx: vscode.ExtensionContext) {
832-
// TODO(hyangah): this is to clean up the unused key. Delete this code in 2021 Dec.
833-
if (ctx.globalState.get('toolsGoInfo')) {
834-
ctx.globalState.update('toolsGoInfo', null);
894+
const configuredGoVersion = await getGoVersion();
895+
if (!configuredGoVersion || configuredGoVersion.lt('1.12')) {
896+
// User is using an ancient or a dev version of go. Don't suggest updates -
897+
// user should know what they are doing.
898+
return;
899+
}
900+
901+
const allTools = getConfiguredTools(configuredGoVersion, getGoConfig(), getGoplsConfig());
902+
const toolsToUpdate = await listOutdatedTools(configuredGoVersion, allTools);
903+
if (toolsToUpdate.length > 0) {
904+
const updateToolsCmdText = 'Update tools';
905+
const selected = await vscode.window.showWarningMessage(
906+
`Tools (${toolsToUpdate.map((tool) => tool.name).join(', ')}) need recompiling to work with ${
907+
configuredGoVersion.version
908+
}`,
909+
updateToolsCmdText
910+
);
911+
if (selected === updateToolsCmdText) {
912+
installTools(toolsToUpdate, configuredGoVersion);
913+
}
835914
}
836915
}
837916

src/util.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export class GoVersion {
9696

9797
constructor(public binaryPath: string, public version: string) {
9898
const matchesRelease = /^go version go(\d\.\d+\S*)\s+/.exec(version);
99-
const matchesDevel = /^go version devel (\S+)\s+/.exec(version);
99+
const matchesDevel = /^go version devel go(\d\.\d+\S*)\s+/.exec(version);
100100
if (matchesRelease) {
101101
// note: semver.parse does not work with Go version string like go1.14.
102102
const sv = semver.coerce(matchesRelease[1]);

test/integration/install.test.ts

+93-5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import util = require('util');
2121
import vscode = require('vscode');
2222
import { isInPreviewMode } from '../../src/goLanguageServer';
2323
import { allToolsInformation } from '../../src/goToolsInformation';
24+
import { listOutdatedTools } from '../../src/goMain';
25+
import * as goInstallTools from '../../src/goInstallTools';
26+
import * as utilModule from '../../src/util';
2427

2528
interface installationTestCase {
2629
name: string;
@@ -238,27 +241,112 @@ function shouldRunSlowTests(): boolean {
238241

239242
suite('getConfiguredTools', () => {
240243
test('do not require legacy tools when using language server', async () => {
241-
const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: true }, {});
244+
const configured = getConfiguredTools(
245+
fakeGoVersion('go version go1.15.6 linux/amd64'),
246+
{ useLanguageServer: true },
247+
{}
248+
);
242249
const got = configured.map((tool) => tool.name) ?? [];
243250
assert(got.includes('gopls'), `omitted 'gopls': ${JSON.stringify(got)}`);
244251
assert(!got.includes('guru') && !got.includes('gocode'), `suggested legacy tools: ${JSON.stringify(got)}`);
245252
});
246253

247254
test('do not require gopls when not using language server', async () => {
248-
const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: false }, {});
255+
const configured = getConfiguredTools(
256+
fakeGoVersion('go version go1.15.6 linux/amd64'),
257+
{ useLanguageServer: false },
258+
{}
259+
);
249260
const got = configured.map((tool) => tool.name) ?? [];
250261
assert(!got.includes('gopls'), `suggested 'gopls': ${JSON.stringify(got)}`);
251262
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
252263
});
253264

254265
test('do not require gopls when the go version is old', async () => {
255-
const configured = getConfiguredTools(fakeGoVersion('1.9'), { useLanguageServer: true }, {});
266+
const configured = getConfiguredTools(
267+
fakeGoVersion('go version go1.9 linux/amd64'),
268+
{ useLanguageServer: true },
269+
{}
270+
);
256271
const got = configured.map((tool) => tool.name) ?? [];
257272
assert(!got.includes('gopls'), `suggested 'gopls' for old go: ${JSON.stringify(got)}`);
258273
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
259274
});
260275
});
261276

262-
function fakeGoVersion(version: string) {
263-
return new GoVersion('/path/to/go', `go version go${version} windows/amd64`);
277+
function fakeGoVersion(versionStr: string) {
278+
return new GoVersion('/path/to/go', versionStr);
264279
}
280+
281+
suite('listOutdatedTools', () => {
282+
let sandbox: sinon.SinonSandbox;
283+
setup(() => {
284+
sandbox = sinon.createSandbox();
285+
});
286+
teardown(() => sandbox.restore());
287+
288+
async function runTest(goVersion: string | undefined, tools: { [key: string]: string | undefined }) {
289+
const binPathStub = sandbox.stub(utilModule, 'getBinPath');
290+
const versionStub = sandbox.stub(goInstallTools, 'inspectGoToolVersion');
291+
for (const tool in tools) {
292+
binPathStub.withArgs(tool).returns(`/bin/${tool}`);
293+
versionStub.withArgs(`/bin/${tool}`).returns(Promise.resolve({ goVersion: tools[tool] }));
294+
}
295+
296+
const toolsToCheck = Object.keys(tools).map((tool) => getToolAtVersion(tool));
297+
298+
const configuredGoVersion = goVersion ? fakeGoVersion(goVersion) : undefined;
299+
const toolsToUpdate = await listOutdatedTools(configuredGoVersion, toolsToCheck);
300+
return toolsToUpdate.map((tool) => tool.name).filter((tool) => !!tool);
301+
}
302+
303+
test('minor version difference requires updates', async () => {
304+
const x = await runTest('go version go1.18 linux/amd64', {
305+
gopls: 'go1.16', // 1.16 < 1.18
306+
dlv: 'go1.17', // 1.17 < 1.18
307+
staticcheck: 'go1.18', // 1.18 == 1.18
308+
gotests: 'go1.19' // 1.19 > 1.18
309+
});
310+
assert.deepStrictEqual(x, ['gopls', 'dlv']);
311+
});
312+
test('patch version difference does not require updates', async () => {
313+
const x = await runTest('go version go1.16.1 linux/amd64', {
314+
gopls: 'go1.16', // 1.16 < 1.16.1
315+
dlv: 'go1.16.1', // 1.16.1 == 1.16.1
316+
staticcheck: 'go1.16.2', // 1.16.2 > 1.16.1
317+
gotests: 'go1.16rc1' // 1.16rc1 != 1.16.1
318+
});
319+
assert.deepStrictEqual(x, ['gotests']);
320+
});
321+
test('go is beta version', async () => {
322+
const x = await runTest('go version go1.18beta2 linux/amd64', {
323+
gopls: 'go1.17.1', // 1.17.1 < 1.18beta2
324+
dlv: 'go1.18beta1', // 1.18beta1 != 1.18beta2
325+
staticcheck: 'go1.18beta2', // 1.18beta2 == 1.18beta2
326+
gotests: 'go1.18' // 1.18 > 1.18beta2
327+
});
328+
assert.deepStrictEqual(x, ['gopls', 'dlv']);
329+
});
330+
test('go is dev version', async () => {
331+
const x = await runTest('go version devel go1.18-41f485b9a7 linux/amd64', {
332+
gopls: 'go1.17.1',
333+
dlv: 'go1.18beta1',
334+
staticcheck: 'go1.18',
335+
gotests: 'go1.19'
336+
});
337+
assert.deepStrictEqual(x, []);
338+
});
339+
test('go is unknown version', async () => {
340+
const x = await runTest('', {
341+
gopls: 'go1.17.1'
342+
});
343+
assert.deepStrictEqual(x, []);
344+
});
345+
test('tools are unknown versions', async () => {
346+
const x = await runTest('go version go1.17 linux/amd64', {
347+
gopls: undefined, // this can be because gopls was compiled with go1.18 or it's too old.
348+
dlv: 'go1.16.1'
349+
});
350+
assert.deepStrictEqual(x, ['dlv']);
351+
});
352+
});

test/integration/utils.test.ts

+2-8
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,10 @@ suite('utils Tests', () => {
2929
test('build GoVersion', () => {
3030
// [input, wantFormat, wantFormatIncludePrerelease, wantIsValid]
3131
const testCases: [string | undefined, string, string, boolean][] = [
32-
[
33-
'go version devel +a295d59d Fri Jun 26 19:00:25 2020 +0000 darwin/amd64',
34-
'devel +a295d59d',
35-
'devel +a295d59d',
36-
true
37-
],
3832
[
3933
'go version devel go1.17-756fd56bbf Thu Apr 29 01:15:34 2021 +0000 darwin/amd64',
40-
'devel go1.17-756fd56bbf',
41-
'devel go1.17-756fd56bbf',
34+
'devel 1.17-756fd56bbf',
35+
'devel 1.17-756fd56bbf',
4236
true
4337
],
4438
['go version go1.14 darwin/amd64', '1.14.0', '1.14', true],

0 commit comments

Comments
 (0)