[vscode-go] src/goLint: switch the default lint tool to staticcheck
44 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Mar 9, 2021, 1:26:32 AM3/9/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, kokoro, Rebecca Stambler, Suzy Mueller, golang-co...@googlegroups.com
Hyang-Ah Hana Kim submitted this change.
Approvals:
Rebecca Stambler: Looks good to me, approved
Hyang-Ah Hana Kim: Trusted; Run TryBots
kokoro: TryBots succeeded
src/goLint: switch the default lint tool to staticcheck
Golint is unmaintained and effectively deprecated.
So, switch to staticcheck, which is one of the existing linter
options we've provided, and is actively maintained.
Compared to golint, we observed staticcheck provides more useful
signals about the code style and common errors, while it is still
easy to use. Some controversial checks existed in the golint are
turned off by default (ST1000, ST1003, ST1016), or removed. Users
can turn them on if want, by supplying -checks as lintFlags.
Unlike golint, staticcheck reports only compile errors and does
not show other lint errors if compile errors are present. Because
the compile errors will be also detected by the language server
or by the buildOnSave/vetOnSave checks and the extension will
deduplicate them, it may look to users as if the linter wasn't
running - I am not sure how much this will bother users yet.
Fixes golang/vscode-go#189
Updates #38968
Change-Id: Ie9334867aef3673aed01d7eaf757966486178dbc
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/279212
Trust: Hyang-Ah Hana Kim <hya...@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
TryBot-Result: kokoro <noreply...@google.com>
Reviewed-by: Rebecca Stambler <rsta...@golang.org>
---
M .github/workflows/release-nightly.yml
M .github/workflows/test-long-all.yml
M .github/workflows/test-long.yml
M .github/workflows/test-smoke.yml
M build/all.bash
M docs/settings.md
M docs/tools.md
M package.json
M src/goLint.ts
M src/goTools.ts
M test/integration/extension.test.ts
M test/testdata/linterTest/linter_1.go
M test/testdata/linterTest/linter_2.go
13 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/.github/workflows/release-nightly.yml b/.github/workflows/release-nightly.yml
index 0fd28c3..7341c45 100644
--- a/.github/workflows/release-nightly.yml
+++ b/.github/workflows/release-nightly.yml
@@ -57,7 +57,7 @@
go get github.com/sqs/goreturns
go get github.com/uudashr/gopkgs/v2/cmd/gopkgs
go get github.com/zmb3/gogetdoc
- go get golang.org/x/lint/golint
+ go get honnef.co/go/tools/...
go get golang.org/x/tools/cmd/gorename
go get golang.org/x/tools/gopls
go get github.com/cweill/gotests/...
diff --git a/.github/workflows/test-long-all.yml b/.github/workflows/test-long-all.yml
index a999273..6e7521b 100644
--- a/.github/workflows/test-long-all.yml
+++ b/.github/workflows/test-long-all.yml
@@ -53,7 +53,7 @@
go get github.com/sqs/goreturns
go get github.com/uudashr/gopkgs/v2/cmd/gopkgs
go get github.com/zmb3/gogetdoc
- go get golang.org/x/lint/golint
+ go get honnef.co/go/tools/...
go get golang.org/x/tools/cmd/gorename
go get golang.org/x/tools/gopls
go get github.com/cweill/gotests/...
diff --git a/.github/workflows/test-long.yml b/.github/workflows/test-long.yml
index 4d6119b..3070067 100644
--- a/.github/workflows/test-long.yml
+++ b/.github/workflows/test-long.yml
@@ -52,7 +52,7 @@
go get github.com/sqs/goreturns
go get github.com/uudashr/gopkgs/v2/cmd/gopkgs
go get github.com/zmb3/gogetdoc
- go get golang.org/x/lint/golint
+ go get honnef.co/go/tools/...
go get golang.org/x/tools/cmd/gorename
go get golang.org/x/tools/gopls
go get github.com/cweill/gotests/...
diff --git a/.github/workflows/test-smoke.yml b/.github/workflows/test-smoke.yml
index 9dcb91c..e314ecd 100644
--- a/.github/workflows/test-smoke.yml
+++ b/.github/workflows/test-smoke.yml
@@ -50,7 +50,7 @@
go get github.com/sqs/goreturns
go get github.com/uudashr/gopkgs/v2/cmd/gopkgs
go get github.com/zmb3/gogetdoc
- go get golang.org/x/lint/golint
+ go get honnef.co/go/tools/...
go get golang.org/x/tools/cmd/gorename
go get golang.org/x/tools/gopls
go get github.com/cweill/gotests/...
diff --git a/build/all.bash b/build/all.bash
index a13776f..fa80da7 100755
--- a/build/all.bash
+++ b/build/all.bash
@@ -115,7 +115,7 @@
GO111MODULE=on go get github.com/sqs/goreturns
GO111MODULE=on go get github.com/uudashr/gopkgs/v2/cmd/gopkgs
GO111MODULE=on go get github.com/zmb3/gogetdoc
- GO111MODULE=on go get golang.org/x/lint/golint
+ GO111MODULE=on go get honnef.co/go/tools/...
GO111MODULE=on go get golang.org/x/tools/cmd/gorename
GO111MODULE=on go get github.com/go-delve/delve/cmd/dlv@master
}
diff --git a/docs/settings.md b/docs/settings.md
index 9f53602..36ef931 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -312,9 +312,9 @@
### `go.lintTool`
Specifies Lint tool name.<br/>
-Allowed Options: `golint`, `golangci-lint`, `revive`, `staticcheck`
+Allowed Options: `staticcheck`, `golint`, `golangci-lint`, `revive`
-Default: `"golint"`
+Default: `"staticcheck"`
### `go.liveErrors`
Use gotype on the file currently being edited and report any semantic or syntactic errors found after configured delay. Not applicable when using the language server.
diff --git a/docs/tools.md b/docs/tools.md
index fc3f9ce..2cf6201 100644
--- a/docs/tools.md
+++ b/docs/tools.md
@@ -60,13 +60,14 @@
This tool provides support for the [`Go: Generate Unit Tests`](features.md#generate-unit-tests) set of commands.
-### [`golint`]
+### [`staticcheck`]
-This is the default lint tool. Other lint tools can be used by configuring the [`"go.lintTool"`](settings.md#go.lintTool) setting. Other options include:
+This is the default lint tool. See the [full list of checks](https://staticcheck.io/docs/checks) that `staticcheck` provides. Other lint tools can be used by configuring the [`"go.lintTool"`](settings.md#go.lintTool) setting.
+Other options include:
- * [`staticcheck`]: This tool provides a great deal of useful checks that are not provided by [`golint`]. See the full list at [staticcheck.io/docs/checks](https://staticcheck.io/docs/checks). It is also officially supported by the [Go team at Google](https://staticcheck.io/sponsors).
- * [`golangci-lint`]: This tool combines a number of existing lint tools, including [staticcheck](#staticcheck), into one interface.
+ * [`golangci-lint`]: This meta-linter combines a number of existing lint tools, including [staticcheck](#staticcheck), into one interface.
* [`revive`]: This tool is an enhancement on top of [`golint`], and it provides additional checks.
+ * [`golint`]: This used to be the default linter used by this extension before it was officially deprecated.
You can use the [`"go.lintFlags"`](settings.md#go.lintFlags) setting to further configure your linter of choice. Most linters can be configured via special configuration files, but you may still need to pass these command-line flags. The configuration documentation for each supported linter is listed here:
diff --git a/package.json b/package.json
index 1f9d8e2..393fb6c 100644
--- a/package.json
+++ b/package.json
@@ -977,14 +977,14 @@
},
"go.lintTool": {
"type": "string",
- "default": "golint",
+ "default": "staticcheck",
"description": "Specifies Lint tool name.",
"scope": "resource",
"enum": [
+ "staticcheck",
"golint",
"golangci-lint",
- "revive",
- "staticcheck"
+ "revive"
]
},
"go.lintFlags": {
diff --git a/src/goLint.ts b/src/goLint.ts
index cad38de..685c644 100644
--- a/src/goLint.ts
+++ b/src/goLint.ts
@@ -9,14 +9,7 @@
import { toolExecutionEnvironment } from './goEnv';
import { lintDiagnosticCollection } from './goMain';
import { diagnosticsStatusBarItem, outputChannel } from './goStatus';
-import {
- getToolsGopath,
- getWorkspaceFolderPath,
- handleDiagnosticErrors,
- ICheckResult,
- resolvePath,
- runTool
-} from './util';
+import { getWorkspaceFolderPath, handleDiagnosticErrors, ICheckResult, resolvePath, runTool } from './util';
/**
* Runs linter on the current file, package or workspace.
*/
@@ -81,7 +74,7 @@
return Promise.resolve([]);
}
- const lintTool = goConfig['lintTool'] || 'golint';
+ const lintTool = goConfig['lintTool'] || 'staticcheck';
const lintFlags: string[] = goConfig['lintFlags'] || [];
const lintEnv = toolExecutionEnvironment();
const args: string[] = [];
@@ -102,16 +95,6 @@
}
args.push(flag);
});
- if (lintTool === 'gometalinter') {
- if (args.indexOf('--aggregate') === -1) {
- args.push('--aggregate');
- }
- if (goConfig['toolsGopath']) {
- // gometalinter will expect its linters to be in the GOPATH
- // So add the toolsGopath to GOPATH
- lintEnv['GOPATH'] += path.delimiter + getToolsGopath();
- }
- }
if (lintTool === 'golangci-lint') {
if (args.indexOf('run') === -1) {
args.unshift('run');
diff --git a/src/goTools.ts b/src/goTools.ts
index 6e9c64a..a4ec994 100644
--- a/src/goTools.ts
+++ b/src/goTools.ts
@@ -343,7 +343,7 @@
name: 'golint',
importPath: 'golang.org/x/lint/golint',
replacedByGopls: false,
- isImportant: true,
+ isImportant: false,
description: 'Linter',
minimumGoVersion: semver.coerce('1.9')
},
diff --git a/test/integration/extension.test.ts b/test/integration/extension.test.ts
index 0d28d30..cd69f9d 100644
--- a/test/integration/extension.test.ts
+++ b/test/integration/extension.test.ts
@@ -425,8 +425,11 @@
const warnings = await goLint(
file2.uri,
Object.create(getGoConfig(), {
- lintTool: { value: 'golint' },
- lintFlags: { value: [] }
+ lintTool: { value: 'staticcheck' },
+ lintFlags: { value: ['-checks', 'all,-ST1000,-ST1016'] }
+ // staticcheck skips debatable checks such as ST1003 by default,
+ // but this test depends on ST1003 (MixedCaps package name) presented in both files
+ // in the same package. So, enable that.
}),
'package'
);
@@ -448,15 +451,17 @@
vetOnSave: { value: 'package' },
vetFlags: { value: ['-all'] },
lintOnSave: { value: 'package' },
- lintTool: { value: 'golint' },
+ lintTool: { value: 'staticcheck' },
lintFlags: { value: [] },
buildOnSave: { value: 'package' }
});
const expectedLintErrors = [
+ // Unlike golint, staticcheck will report only those compile errors,
+ // but not lint errors when the program is broken.
{
- line: 7,
+ line: 11,
severity: 'warning',
- msg: 'exported function Print2 should have comment or be unexported'
+ msg: 'undeclared name: prin (compile)'
}
];
// If a user has enabled diagnostics via a language server,
@@ -466,6 +471,7 @@
? []
: [{ line: 11, severity: 'error', msg: 'undefined: prin' }];
+ // `check` itself doesn't run deDupeDiagnostics, so we expect all vet/lint errors.
const expected = [...expectedLintErrors, ...expectedBuildVetErrors];
const diagnostics = await check(vscode.Uri.file(path.join(fixturePath, 'errorsTest', 'errors.go')), config);
const sortedDiagnostics = ([] as ICheckResult[]).concat
diff --git a/test/testdata/linterTest/linter_1.go b/test/testdata/linterTest/linter_1.go
index b76bee3..1c42905 100644
--- a/test/testdata/linterTest/linter_1.go
+++ b/test/testdata/linterTest/linter_1.go
@@ -1,12 +1,17 @@
package linterTest
-import "fmt"
+import (
+ "fmt"
+)
func ExportedFunc() {
- a := 10
- func() {
- a := 20
- }()
-
- fmt.Println("OUTER A: ", a)
+ x := compute()
+ if x == nil {
+ fmt.Println("nil pointer received")
+ }
+ // if x is nil, the next line will panic.
+ foo(*x)
}
+
+func compute() **int { return nil }
+func foo(x *int) { fmt.Println(*x) }
diff --git a/test/testdata/linterTest/linter_2.go b/test/testdata/linterTest/linter_2.go
index f58961a..33916a5 100644
--- a/test/testdata/linterTest/linter_2.go
+++ b/test/testdata/linterTest/linter_2.go
@@ -1,5 +1,10 @@
package linterTest
-func secondFunc() error {
+import (
+ "errors"
+ "fmt"
+)
+func secondFunc() error {
+ return errors.New(fmt.Sprint("Errors"))
}
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
The name of the file: docs/tools.md
Insertions: 1, Deletions: 1.
```
@@ -69:70, +69:70 @@
- * [`golint`]: This used to be the default linter used by this extension before it was officially deprecated.
+ * [`golint`]: This used to be the default linter used by this extension before it was officially deprecated.
```
To view, visit change 279212. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages