Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usability: Unconfigured gopls diagnostics unionise with configured golangci-lint restults. #1218

Closed
brackendawson opened this issue Feb 9, 2021 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@brackendawson
Copy link

brackendawson commented Feb 9, 2021

What version of Go, VS Code & VS Code Go extension are you using?

  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.15.7 darwin/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.6.5
      golang.org/x/tools/[email protected] h1:kLt9rD/dWtVdvc8LmdcxagDFih6zxYXREpKSYYZu5KE=
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.48.2
      a0479759d6e9ea56afa657e454193f72aef85bd0
      x64
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.22.1
  • golangci-lint version:
    • golangci-lint has version 1.23.8 built from 76a82c6 on 2020-03-04T14:27:24Z
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
Expand...Checking configured tools.... GOBIN: /Users/alandaws/bin toolsGopath: gopath: /Users/alandaws GOROOT: /usr/local/go PATH: /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/usr/local/go/bin:/Users/alandaws/bin

gopkgs: /Users/alandaws/bin/gopkgs installed
go-outline: /Users/alandaws/bin/go-outline installed
gotests: /Users/alandaws/bin/gotests installed
gomodifytags: /Users/alandaws/bin/gomodifytags installed
impl: /Users/alandaws/bin/impl installed
goplay: /Users/alandaws/bin/goplay installed
dlv: /Users/alandaws/bin/dlv installed
golangci-lint: /Users/alandaws/bin/golangci-lint installed
gopls: /Users/alandaws/bin/gopls installed

go env
Workspace Folder (registry-cli): /Users/alandaws/src/github.ibm.com/alchemy-registry/registry-cli
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/alandaws/bin"
GOCACHE="/Users/alandaws/Library/Caches/go-build"
GOENV="/Users/alandaws/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/alandaws/pkg/mod"
GONOPROXY="github.ibm.com"
GONOSUMDB="github.ibm.com"
GOOS="darwin"
GOPATH="/Users/alandaws"
GOPRIVATE="github.ibm.com"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/alandaws/src/github.ibm.com/alchemy-registry/registry-cli/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/06/jjq25lws0cbcxv37ns0hb_mm0000gn/T/go-build687529871=/tmp/go-build -gno-record-gcc-switches -fno-common"

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

{
    "go.formatTool": "goimports",
    "go.coverageOptions": "showUncoveredCodeOnly",
    "go.testOnSave": true,
    "go.gocodeAutoBuild": true,
    "go.gotoSymbol.includeImports": true,
    "go.gotoSymbol.includeGoroot": true,
    "go.testEnvFile": "",
    "go.coverOnSave": true,
    "go.lintTool": "golangci-lint",
    "go.lintOnSave": "workspace",
    "go.lintFlags": [
        "--config=${workspaceFolder}/.golangci.yml" // required if `go.lintOnSave` is set to `package`
    ],
    "go.languageServerFlags": [
    ],
    // "go.languageServerExperimentalFeatures.diagnostics": false. // this is a workaround to this issue
}

.golangci.yml:

run:
  tests: false
  skip-dirs:
    - testutils
    - i18n

linters:
  enable:
    - gosec
    - godox
    - golint
    - interfacer
    - unconvert
    - goimports
    - misspell
    - prealloc
    - gocritic

Describe the bug

If the user enables go.lintOnSave at some level, with the go.lintTool set to golangci-lint, and they use a .golangci.yml configuration; then the "Problems" tab will seemingly ignore some of their configurations. It will show problems in test files if run.tests is disabled, for example.

Steps to reproduce the behavior:

  1. Open a go module in a workspace with the above .golanci.yaml.
  2. Use the above vscode-go settings.
  3. Introduce a lint error to a test file. For example, define a redundant type in a composite literal:
var Problem [][]string{
    []string{"my type is redundant"}
}
  1. Save and witness problems shown for the test file even though test files are excluded.

Diagnosis

This happens because the Problems tab is now the union of the golangci-lint results and the gopls diagnostics.

Workaround

As a workaround you can set go.languageServerExperimentalFeatures.diagnostics to false to show only the golangci-lint problems.

Suggestions

The "Vet On Save" setting includes a note to tell you that this setting is overridden when using the language server, and tells you how to configure this for the language server:
image

Perhaps the lint settings can be given a similar note in their descriptions to let users know how to disable the language server's checks?

Or perhaps the default linter should now be none and enabling one should implicitly disable the language server diagnostics?

@brackendawson brackendawson changed the title Usability: Unconfigured gopls diagnostics intersect with configured golangci-lint restults. Usability: Unconfigured gopls diagnostics unionise with configured golangci-lint restults. Feb 9, 2021
@hyangah
Copy link
Contributor

hyangah commented Feb 9, 2021

@brackendawson The extension dedups the diagnostics from linter and the language server (Issue 142).

It sounds like what you want is to completely disable basic diagnostics from the language server.
@stamblerre can we add an option to completely disable diagnostics from the language server in the context of #50?

Disabling language server's diagnostics just because the user configured a linter will be a breaking change - some users would use a linter to run custom checks in addition to the vet, compile, staticchecks, and other analysis checks offered by the language server.

@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 9, 2021
@stamblerre
Copy link
Contributor

@stamblerre can we add an option to completely disable diagnostics from the language server in the context of #50?

Sure--that is one of the suggested possibilities in that issue. @brackendawson: Please leave a reaction on #50 if you think it would fit your use case.

Disabling language server's diagnostics just because the user configured a linter will be a breaking change - some users would use a linter to run custom checks in addition to the vet, compile, staticchecks, and other analysis checks offered by the language server.

I don't think we should consider doing this. Linters provide fundamentally different types of checks from gopls--I imagine that the majority of users want to know both if their code builds, no matter how they configure their linter to work. Of course, in this case, the issue is that gopls shows lint-style diagnostics (from gofmt -s). Individual checks like this can be disabled using the analyses setting, but gopls doesn't have the granular configuration necessary to disable these checks only in test files. In that case, I think requiring the user to manually disable gopls diagnostics is an acceptable solution.

@brackendawson
Copy link
Author

An option to disable gopls diagnostics fits my use case. I think anyone looking to configure a linter will notice that checkbox and understand that it can also put things in the Problems tab.
Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants