Skip to content

Commit

Permalink
extension/src/goTools.ts: remove 'replacedByGopls' field
Browse files Browse the repository at this point in the history 
FORCE RUN CI

This field was added when we were deprecating several tools with gopls
gradually and we wanted to keep the complexity of `getConfiguredTools`
manageable. Now we have only a handful number of tools. Remove it.

Also, remove the custom formatter check in getConfiguredTools.
The idea was to add a custom formatter tool to the configured tools list
if the user configured "go.formatTool": "custom". But, since custom tool
name won't be in `allToolsInformation`, so the custom tool won't be
included in the list anyway. (We install only tools known to us).

Change-Id: I4b26d3756fe45e1cc29f8434b1b5d83ee8977d47
  • Loading branch information
@hyangah
hyangah committed Oct 10, 2024
1 parent 54908e0 commit bc99bc2
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 39 deletions.
2 changes: 1 addition & 1 deletion extension/src/goLint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function goLint(
scope?: string
): Promise<ICheckResult[]> {
const lintTool = goConfig['lintTool'] || 'staticcheck';
if (lintTool === 'staticcheck' && goplsStaticcheckEnabled(goConfig, goplsConfig)) {
if (lintTool === 'staticcheck' && goplsStaticcheckEnabled(goplsConfig)) {
return Promise.resolve([]);
}

Expand Down
19 changes: 6 additions & 13 deletions extension/src/goTools.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import moment = require('moment');
import semver = require('semver');
import { getFormatTool, usingCustomFormatTool } from './language/legacy/goFormat';
import { getFormatTool } from './language/legacy/goFormat';
import { allToolsInformation } from './goToolsInformation';
import { GoVersion } from './util';

Expand All @@ -17,7 +17,6 @@ export interface Tool {
importPath: string;
modulePath: string;
isImportant: boolean;
replacedByGopls?: boolean;
description: string;

// If true, consider prerelease version in prerelease mode
Expand Down Expand Up @@ -121,9 +120,7 @@ export function getConfiguredTools(goConfig: { [key: string]: any }, goplsConfig
function maybeAddTool(name: string) {
const tool = allToolsInformation[name];
if (tool) {
if (!useLanguageServer || !tool.replacedByGopls) {
tools.push(tool);
}
tools.push(tool);
}
}

Expand All @@ -144,25 +141,21 @@ export function getConfiguredTools(goConfig: { [key: string]: any }, goplsConfig
maybeAddTool('dlv');
}

// Only add format tools if the language server is disabled or the
// format tool is known to us.
if (!useLanguageServer || usingCustomFormatTool(goConfig)) {
// Only add format tools if the language server is disabled.
if (!useLanguageServer) {
maybeAddTool(getFormatTool(goConfig));
}

// 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);
const goplsStaticheckEnabled = useLanguageServer && goplsStaticcheckEnabled(goplsConfig);
if (goConfig['lintTool'] !== 'staticcheck' || !goplsStaticheckEnabled) {
maybeAddTool(goConfig['lintTool']);
}
return tools;
}

export function goplsStaticcheckEnabled(
goConfig: { [key: string]: any },
goplsConfig: { [key: string]: any }
): boolean {
export function goplsStaticcheckEnabled(goplsConfig: { [key: string]: any }): boolean {
if (
goplsConfig['ui.diagnostic.staticcheck'] === false ||
(goplsConfig['ui.diagnostic.staticcheck'] === undefined && goplsConfig['staticcheck'] !== true)
Expand Down
12 changes: 0 additions & 12 deletions extension/src/goToolsInformation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gomodifytags',
importPath: 'github.com/fatih/gomodifytags',
modulePath: 'github.com/fatih/gomodifytags',
replacedByGopls: false,
isImportant: false,
description: 'Modify tags on structs',
defaultVersion: 'v1.17.0'
Expand All @@ -18,7 +17,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'goplay',
importPath: 'github.com/haya14busa/goplay/cmd/goplay',
modulePath: 'github.com/haya14busa/goplay',
replacedByGopls: false,
isImportant: false,
description: 'The Go playground',
defaultVersion: 'v1.0.0'
Expand All @@ -27,7 +25,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'impl',
importPath: 'github.com/josharian/impl',
modulePath: 'github.com/josharian/impl',
replacedByGopls: false,
isImportant: false,
description: 'Stubs for interfaces',
defaultVersion: 'v1.4.0'
Expand All @@ -36,7 +33,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gofumpt',
importPath: 'mvdan.cc/gofumpt',
modulePath: 'mvdan.cc/gofumpt',
replacedByGopls: true,
isImportant: false,
description: 'Formatter',
defaultVersion: 'v0.7.0'
Expand All @@ -45,15 +41,13 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'goimports',
importPath: 'golang.org/x/tools/cmd/goimports',
modulePath: 'golang.org/x/tools',
replacedByGopls: true,
isImportant: true,
description: 'Formatter'
},
'gotests': {
name: 'gotests',
importPath: 'github.com/cweill/gotests/gotests',
modulePath: 'github.com/cweill/gotests',
replacedByGopls: false,
isImportant: false,
description: 'Generate unit tests',
minimumGoVersion: semver.coerce('1.9'),
Expand All @@ -64,7 +58,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'golint',
importPath: 'golang.org/x/lint/golint',
modulePath: 'golang.org/x/lint',
replacedByGopls: false,
isImportant: false,
description: 'Linter',
minimumGoVersion: semver.coerce('1.9')
Expand All @@ -73,15 +66,13 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'staticcheck',
importPath: 'honnef.co/go/tools/cmd/staticcheck',
modulePath: 'honnef.co/go/tools',
replacedByGopls: false,
isImportant: true,
description: 'Linter'
},
'golangci-lint': {
name: 'golangci-lint',
importPath: 'github.com/golangci/golangci-lint/cmd/golangci-lint',
modulePath: 'github.com/golangci/golangci-lint',
replacedByGopls: false,
isImportant: true,
description: 'Linter',
minimumGoVersion: semver.coerce('1.20')
Expand All @@ -98,7 +89,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gopls',
importPath: 'golang.org/x/tools/gopls',
modulePath: 'golang.org/x/tools/gopls',
replacedByGopls: false, // lol
isImportant: true,
description: 'Language Server from Google',
usePrereleaseInPreviewMode: true,
Expand All @@ -110,7 +100,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'dlv',
importPath: 'github.com/go-delve/delve/cmd/dlv',
modulePath: 'github.com/go-delve/delve',
replacedByGopls: false,
isImportant: false,
description: 'Go debugger (Delve)',
latestVersion: semver.parse('v1.8.3'),
Expand All @@ -121,7 +110,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'vscgo',
importPath: 'github.com/golang/vscode-go/vscgo',
modulePath: 'github.com/golang/vscode-go/vscgo',
replacedByGopls: false,
isImportant: false, // TODO: set to true when we need it
description: 'VS Code Go helper program',
minimumGoVersion: semver.coerce('1.18')
Expand Down
22 changes: 21 additions & 1 deletion extension/test/integration/install.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,32 @@ suite('getConfiguredTools', () => {
const got = configured.map((tool) => tool.name) ?? [];
assert(got.includes('gopls'), `omitted 'gopls': ${JSON.stringify(got)}`);
});

test('do not require gopls when not using language server', async () => {
const configured = getConfiguredTools({ useLanguageServer: false }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(!got.includes('gopls'), `suggested 'gopls': ${JSON.stringify(got)}`);
});
const hasFormatter = (got: string[]) => {
const formatter = got.filter((tool) =>
['goimports', 'gofmt', 'gofumpt', 'gofumports', 'goreturns'].includes(tool)
);
return formatter?.length > 0;
};
test('do not require a known formatter when using language server', async () => {
const configured = getConfiguredTools({ useLanguageServer: true, formatTool: 'default' }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(!hasFormatter(got), `suggested formatter: ${JSON.stringify(got)}`);
});
test('require a known formatter when not using language server', async () => {
const configured = getConfiguredTools({ useLanguageServer: false, formatTool: 'default' }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(hasFormatter(got), `omitted formatter: ${JSON.stringify(got)}`);
});
test('require a linter', async () => {
const configured = getConfiguredTools({ useLanguageServer: true, lintTool: 'staticcheck' }, {});
const got = configured.map((tool) => tool.name) ?? [];
assert(got.includes('staticcheck'), `omitted 'staticcheck': ${JSON.stringify(got)}`);
});
});

function fakeGoVersion(versionStr: string) {
Expand Down
12 changes: 0 additions & 12 deletions extension/tools/allTools.ts.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gomodifytags',
importPath: 'github.com/fatih/gomodifytags',
modulePath: 'github.com/fatih/gomodifytags',
replacedByGopls: false,
isImportant: false,
description: 'Modify tags on structs',
defaultVersion: 'v1.17.0'
Expand All @@ -16,7 +15,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'goplay',
importPath: 'github.com/haya14busa/goplay/cmd/goplay',
modulePath: 'github.com/haya14busa/goplay',
replacedByGopls: false,
isImportant: false,
description: 'The Go playground',
defaultVersion: 'v1.0.0'
Expand All @@ -25,7 +23,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'impl',
importPath: 'github.com/josharian/impl',
modulePath: 'github.com/josharian/impl',
replacedByGopls: false,
isImportant: false,
description: 'Stubs for interfaces',
defaultVersion: 'v1.4.0'
Expand All @@ -34,7 +31,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gofumpt',
importPath: 'mvdan.cc/gofumpt',
modulePath: 'mvdan.cc/gofumpt',
replacedByGopls: true,
isImportant: false,
description: 'Formatter',
defaultVersion: 'v0.7.0'
Expand All @@ -43,15 +39,13 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'goimports',
importPath: 'golang.org/x/tools/cmd/goimports',
modulePath: 'golang.org/x/tools',
replacedByGopls: true,
isImportant: true,
description: 'Formatter'
},
'gotests': {
name: 'gotests',
importPath: 'github.com/cweill/gotests/gotests',
modulePath: 'github.com/cweill/gotests',
replacedByGopls: false,
isImportant: false,
description: 'Generate unit tests',
minimumGoVersion: semver.coerce('1.9'),
Expand All @@ -62,7 +56,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'golint',
importPath: 'golang.org/x/lint/golint',
modulePath: 'golang.org/x/lint',
replacedByGopls: false,
isImportant: false,
description: 'Linter',
minimumGoVersion: semver.coerce('1.9')
Expand All @@ -71,15 +64,13 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'staticcheck',
importPath: 'honnef.co/go/tools/cmd/staticcheck',
modulePath: 'honnef.co/go/tools',
replacedByGopls: false,
isImportant: true,
description: 'Linter'
},
'golangci-lint': {
name: 'golangci-lint',
importPath: 'github.com/golangci/golangci-lint/cmd/golangci-lint',
modulePath: 'github.com/golangci/golangci-lint',
replacedByGopls: false,
isImportant: true,
description: 'Linter',
minimumGoVersion: semver.coerce('1.20')
Expand All @@ -96,7 +87,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'gopls',
importPath: 'golang.org/x/tools/gopls',
modulePath: 'golang.org/x/tools/gopls',
replacedByGopls: false, // lol
isImportant: true,
description: 'Language Server from Google',
usePrereleaseInPreviewMode: true,
Expand All @@ -108,7 +98,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'dlv',
importPath: 'github.com/go-delve/delve/cmd/dlv',
modulePath: 'github.com/go-delve/delve',
replacedByGopls: false,
isImportant: false,
description: 'Go debugger (Delve)',
latestVersion: semver.parse('v1.8.3'),
Expand All @@ -119,7 +108,6 @@ export const allToolsInformation: { [key: string]: Tool } = {
name: 'vscgo',
importPath: 'github.com/golang/vscode-go/vscgo',
modulePath: 'github.com/golang/vscode-go/vscgo',
replacedByGopls: false,
isImportant: false, // TODO: set to true when we need it
description: 'VS Code Go helper program',
minimumGoVersion: semver.coerce('1.18')
Expand Down

0 comments on commit bc99bc2

Please sign in to comment.