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

Linting package loses line information for lint issues #743

Closed
A-UNDERSCORE-D opened this issue Oct 5, 2020 · 5 comments
Closed

Linting package loses line information for lint issues #743

A-UNDERSCORE-D opened this issue Oct 5, 2020 · 5 comments

Comments

@A-UNDERSCORE-D
Copy link

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

  • Run go version to get version of Go
    • go version go1.15.2 linux/amd64
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders
    • 1.49.2 -- x64
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.17.2
  • Run go env to get the go development environment details
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/ad/development/go/bin"
GOCACHE="/home/ad/.cache/go-build"
GOENV="/home/ad/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ad/development/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ad/development/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ad/development/go/gopaste/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build741992281=/tmp/go-build -gno-record-gcc-switches"

Share the Go related settings you have added/edited

    "go.useLanguageServer": true,
    "go.languageServerExperimentalFeatures": {
        "diagnostics": true,
        "documentLink": true,
    },
    "gopls": {
        "gofumpt": true,
        "analyses": {
            "fillreturns": true,
            "undeclaredname": true,
            "unusedparams": true,
            "nonewvars": true,
        },
        "staticcheck": true,
    },
    "go.autocompleteUnimportedPackages": true,
    "go.useCodeSnippetsOnFunctionSuggest": true,
    "go.delveConfig": {
        "dlvLoadConfig": {
            "followPointers": true,
        },
    },
    "go.buildOnSave": "workspace",
    "go.testFlags": [
        "-race",
        "-v"
    ],
    "go.addTags": {
        "tags": "toml",
        "promptForTags": true,
        "options": "",
        "transform": "snakecase"
    },
    "go.removeTags": {
        "tags": "",
        "options": "",
        "promptForTags": true
    },
    "go.formatTool": "goimports",
    "go.lintFlags": [
        "--fast"
    ],
    "[go]": {
        "editor.codeActionsOnSave": {
            "source.fixAll": true
        }
    },

Describe the bug

Lint issue location on line (as in from index x to y, for underlining. See image below)
I would expect both issue existence and line location to be kept.

Steps to reproduce the behavior:

  1. Open multi-file project with issues in multiple files
  2. Open two or more files, both of which with lint issues
  3. Save (or otherwise trigger linters)
  4. Switch to another file, note that any issues are underlined incorrectly (only the whitespace preceding the line is underlined)

Screenshots or recordings

Expected behavior (file was linted with that file's tab open)
Screenshot from 2020-10-05 15-30-37

Actual behavior when linting another file and this one gets caught up with it:
Screenshot from 2020-10-05 15-30-27

@hyangah
Copy link
Contributor

hyangah commented Oct 5, 2020

@A-UNDERSCORE-D Thanks for the report - what lint tool are you using?

I couldn't reproduce this issue with the default golint tool yet. Based on your settings ("go.lintFlags": [ "--fast"] which isn't recognized by golint, I guess you are using other linter.

@A-UNDERSCORE-D
Copy link
Author

golangci-lint. Sorry apparently I missed it in the config, my settings.json isnt organized well.

relevant snippets:

    "go.lintFlags": [
        "--fast"
    ],
    "go.lintTool": "golangci-lint",

and a part I missed:

    "go.editorContextMenuCommands": {
        "fillStruct": true,
        "toggleTestFile": true,
        "addTags": true,
        "removeTags": false,
        "testAtCursor": true,
        "testFile": false,
        "testPackage": false,
        "generateTestForFunction": true,
        "generateTestForFile": false,
        "generateTestForPackage": false,
        "addImport": true,
        "testCoverage": true,
        "playground": true,
        "debugTestAtCursor": true,
    },

For the linter itself, here is its config file, just in case Ive missed something in their code. I checked the debug output before I posted and it looked the same for both, so Im assuming its something your side

linters-settings:
  govet:
    check-shadowing: true

  gocognit:
    min-complexity: 15

  maligned:
    suggest-new: true

  misspell:
    locale: GB

  gocritic: # cSpell: disable-line
    enabled-tags:
      - diagnostic
      - experimental
      - opinionated
      - performance
      - style

linters:
  disable-all: true
  # cSpell: disable
  enable:
    - megacheck
    - deadcode
    - dogsled
    - dupl
    - errcheck
    - gocognit
    - goconst
    - gocritic
    - gocyclo
    - gosec
    - govet
    - golint
    - gofmt
    - ineffassign
    - lll
    - maligned
    - nakedret
    - scopelint
    - structcheck
    - typecheck
    - unconvert
    - unparam
    - varcheck
    - whitespace
    - wsl

    - misspell
    - funlen
    - bodyclose
    - goprintffuncname
    - interfacer
    - nakedret
    - godox
    - goerr113
    - nestif
    - nolintlint
    - gofumpt

run:
  max-issues-per-linter: 0

  skip-dirs:
    - internal/transport/network

  skip-dirs-use-default: true

issues:
  exclude-use-default: false
  exclude:
    # errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
    - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked

    # Unmarshal XML and JSON are obvious in what they do. Lets not.
    - exported method `.+\.Unmarshal(?:XML|JSON)` should have comment or be unexported
    # golint: Annoying issue about not having a comment. The rare codebase has such comments
    # - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)

    # golint: False positive when tests are defined in package 'test'
    - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this

    # govet: Common false positives
    - (possible misuse of unsafe.Pointer|should have signature)

    # staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
    # - ineffective break statement. Did you mean to break out of the outer loop

    # gosec: Too many false-positives on 'unsafe' usage
    - Use of unsafe calls should be audited

    # gosec: Too many false-positives for parametrized shell calls
    - Subprocess launch(ed with variable|ing should be audited)

    # gosec: Duplicated errcheck checks
    - G104

    # gosec: Too many issues in popular repos
    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)

    # gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)'
    - Potential file inclusion via variable

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/259797 mentions this issue: src/util.ts: handle diagnostics errors for all visible files

@hyangah
Copy link
Contributor

hyangah commented Oct 6, 2020

@A-UNDERSCORE-D thanks for the complete reproducible example.

As described in the cl/259797 commit message, this problem is primarily caused by the fact that the underlying tools do not provide the complete error range info and the extension had to reconstruct reasonable ranges vscode would like. I couldn't find a reasonable workaround for complete fix yet but I hope this problem becomes less severe as underlying tools (e.g. gopls) for linting/vetting provide complete range infos and vscode do not need this type of hack.

@A-UNDERSCORE-D
Copy link
Author

Yeah I gave it a read earlier. Thanks for trying :D Hopefully the backends get a bit better about it. Until then I'm sure your fix will suffice.

@golang golang locked and limited conversation to collaborators Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants