golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.65k stars 1.39k forks source link

nonolint fix actual nolint:exhaustive #1940

Open Antonboom opened 3 years ago

Antonboom commented 3 years ago
Description of the problem `--fix` remove actual `nolint` directive.
Version of golangci-lint ```console $ golangci-lint --version golangci-lint has version 1.39.0 built from 9aea4ae on 2021-03-26T10:52:04Z ```
Config file ```console $ cat .golangci.yml run: tests: true skip-dirs: - "demo" - "cmd/fsm-graph" skip-files: - ".*\\.pb\\.go$" - ".*\\.pb\\.gw\\.go$" - ".*generated_.*\\.go$" - ".*_generated\\.go$" - "cmd/matrix-switcher/.*" deadline: 3m linters-settings: errcheck: check-type-assertions: true check-blank: true exhaustive: default-signifies-exhaustive: false forbidigo: forbid: - fmt.Errorf funlen: lines: 150 statements: 80 gci: local-prefixes: xxx/be gocognit: min-complexity: 100 lll: line-length: 130 nestif: min-complexity: 20 issues: max-same-issues: 0 exclude-rules: - path: main.go linters: - funlen - path: _test\.go linters: - dupl - gocognit - goconst - gocyclo - gosec - funlen - lll - scopelint # frequent false positives in tests https://github.com/kyoh86/scopelint/issues/4 - linters: - lll # https://github.com/walle/lll/pull/13 source: "^//go:generate " - linters: - lll source: "^// https://mipldev.hopto.org" - linters: - deadcode - unused - varcheck source: "\\s+_\\S*" - linters: - structcheck - unused source: "tableName" - linters: - golint # https://github.com/golang/lint/issues/511 source: "_ \"embed\"" linters: disable-all: true enable: - asciicheck - bodyclose - deadcode - depguard - dogsled - dupl - durationcheck - errcheck - exhaustive - exportloopref - forbidigo - funlen - gci - gocognit - goconst - gocritic - gocyclo - godot - gofmt - gofumpt - goheader - goimports - golint - gomodguard - gomoddirectives - goprintffuncname - gosec - gosimple - govet - ifshort - ineffassign - importas - lll - makezero - misspell - nestif - nilerr - noctx - nolintlint - prealloc - predeclared - rowserrcheck - sqlclosecheck - staticcheck - structcheck - stylecheck - tparallel - typecheck - unconvert - unparam - unused - varcheck - wastedassign - whitespace ```
Go environment ```console $ go version && go env GO111MODULE="" GOARCH="amd64" GOBIN="/Users/a.telyshev/go/bin" GOCACHE="/Users/a.telyshev/Library/Caches/go-build" GOENV="/Users/a.telyshev/Library/Application Support/go/env" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/a.telyshev/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/a.telyshev/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/Cellar/go/1.16.3/libexec" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/Cellar/go/1.16.3/libexec/pkg/tool/darwin_amd64" GOVCS="" GOVERSION="go1.16.3" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/dev/null" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8c/0cgvdsl90d32skq_jttyr_nj1h1jzd/T/go-build1033582149=/tmp/go-build -gno-record-gcc-switches -fno-common" ```
Verbose output of running ```console $ golangci-lint cache clean $ golangci-lint run -v INFO [config_reader] Config search paths: [./ /Users/a.telyshev/softpro/live-be /Users/a.telyshev/softpro /Users/a.telyshev /Users /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 54 linters: [asciicheck bodyclose deadcode depguard dogsled dupl durationcheck errcheck exhaustive exportloopref forbidigo funlen gci gocognit goconst gocritic gocyclo godot gofmt gofumpt goheader goimports golint gomoddirectives gomodguard goprintffuncname gosec gosimple govet ifshort importas ineffassign lll makezero misspell nestif nilerr noctx nolintlint prealloc predeclared rowserrcheck sqlclosecheck staticcheck structcheck stylecheck tparallel typecheck unconvert unparam unused varcheck wastedassign whitespace] INFO [loader] Go packages loading at mode 575 (types_sizes|compiled_files|deps|imports|exports_file|files|name) took 967.784525ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 28.363487ms INFO [linters context/goanalysis] analyzers took 12m40.122265833s with top 10 stages: buildir: 1m33.880349376s, the_only_name: 40.643310299s, buildssa: 27.963043346s, wastedassign: 26.178112115s, dupl: 21.007821627s, forbidigo: 15.295167766s, unconvert: 14.161358185s, exhaustive: 13.985082559s, golint: 13.762227318s, gosec: 13.607533401s INFO [runner/skip dirs] Skipped 5 issues from dir internal/demo/websocket by pattern demo INFO [runner/skip dirs] Skipped 2 issues from dir internal/demo/scanner-hf800 by pattern demo INFO [runner/skip dirs] Skipped 4 issues from dir cmd/fsm-graph by pattern cmd/fsm-graph INFO [runner/skip dirs] Skipped 1 issues from dir internal/demo/oanda by pattern demo INFO [runner] Issues before processing: 7860, after processing: 1 INFO [runner] Processors filtering stat (out/in): path_prettifier: 7860/7860, sort_results: 1/1, diff: 1/1, max_same_issues: 1/1, max_from_linter: 1/1, filename_unadjuster: 7860/7860, skip_files: 1463/7860, autogenerated_exclude: 1446/1451, exclude: 1446/1446, uniq_by_line: 1/1, max_per_file_from_linter: 1/1, source_code: 1/1, path_prefixer: 1/1, skip_dirs: 1451/1463, identifier_marker: 1446/1446, exclude-rules: 92/1446, nolint: 1/92, cgo: 7860/7860, path_shortener: 1/1, severity-rules: 1/1 INFO [runner] processing took 96.548607ms with stages: identifier_marker: 21.513352ms, skip_files: 20.311435ms, path_prettifier: 17.138714ms, exclude-rules: 16.537678ms, nolint: 11.456799ms, autogenerated_exclude: 6.226927ms, skip_dirs: 1.1979ms, cgo: 1.181319ms, filename_unadjuster: 970.609µs, source_code: 5.54µs, uniq_by_line: 1.99µs, max_from_linter: 1.443µs, path_shortener: 1.358µs, max_same_issues: 894ns, diff: 724ns, max_per_file_from_linter: 718ns, severity-rules: 405ns, exclude: 311ns, sort_results: 297ns, path_prefixer: 194ns INFO [runner] linters took 19.918832651s with stages: goanalysis_metalinter: 19.821823884s internal/facade/facade-player/adapters.go:346:2: missing cases in switch of type fsm.StateName: DefaultStateName, EndState (exhaustive) switch s { ^ INFO File cache stats: 666 entries of total size 2.0MiB INFO Memory: 197 samples, avg is 946.4MB, max is 1086.3MB INFO Execution took 20.959312888s ```
Code example or link to a public repository ```go func adaptRPState(s fsmrp.StateName) pbRp.State { switch s { case fsmrp.StateAnte: return pbRp.State_STATE_ANTE_ACCEPTED // ... } return pbRp.State_STATE_UNSPECIFIED } ```

I fell into a trap

internal/facade/facade-player/adapters.go:346:13: directive `//nolint:exhaustive` is unused for linter "exhaustive" (nolintlint)
        switch s { //nolint:exhaustive
internal/facade/facade-player/adapters.go:346:2: missing cases in switch of type fsm.StateName: DefaultStateName, EndState (exhaustive)
        switch s {
Antonboom commented 3 years ago

And magic. Such code linted ok:

func adaptRPState(s fsmrp.StateName) pbRp.State { //nolint:nolintlint
    switch s {
Antonboom commented 3 years ago

After another yet cache cleaning last working variant:

func adaptRPState(s fsmrp.StateName) pbRp.State { //nolint:nolintlint
    switch s { //nolint:exhaustive
ldez commented 3 years ago

Hello,

I created a minimal example:

main.go ```go package main import "fmt" type StateName int const ( Green StateName = iota Yellow Red Blue ) func main() { fmt.Println(AdaptState(Green)) } func AdaptState(s StateName) string { switch s { //nolint:exhaustive case Green: return "Green" case Yellow: return "Yellow" } return "unknown" } ```
.golangci.yml ```yml linters-settings: exhaustive: default-signifies-exhaustive: false linters: disable-all: true enable: - exhaustive - nolintlint ```

And I don't reproduce.

Can you update my example to recreate the problem? Or provide another reproducible example?

ldez commented 3 years ago

@Antonboom any feedback?

Antonboom commented 3 years ago

I prepared more complex example similar to project code.

nonolint.zip

func adaptTigerToInt(t tigers.Cat) int {
    switch t { //nolint:exhaustive
    case tigers.Tiger1:
        return 1
    }
    return 0
}

/*
▶ golangci-lint run .
adapt.go:6:2: missing cases in switch of type cats.Cat: Cat1|Cat2 (exhaustive)
        switch t {
        ^

▶ golangci-lint run .
adapt.go:6:13: directive `//nolint:exhaustive` is unused for linter "exhaustive" (nolintlint)
        switch t { //nolint:exhaustive
*/
Antonboom commented 3 years ago

It seems like exhaustive's issue for type alias supporting.

ldez commented 3 years ago

Yes sounds like an issue with exhaustive, can you open an issue on this repo https://github.com/nishanths/exhaustive

nishanths commented 3 years ago

For anyone arriving here, I've added details in the related issue: https://github.com/nishanths/exhaustive/issues/7#issuecomment-879512515. It appears to be a general issue with golangci-lint not invalidating the cache when source files have changed.

powerman commented 2 years ago

I have this issue on latest version (1.48.0) and I can confirm it's caching issue.

ian-h-chamberlain commented 2 years ago

My org has seen some issues with false-positive or false-negative that we suspect is due to similar caching issues... Does anyone know if there is already a separate open issue for that (I don't see a link here)?

Meanwhile we've just disabled nolintlint but it would be nice to have it working again if we could get deterministic results...

nocive commented 2 years ago

@ian-h-chamberlain https://github.com/golangci/golangci-lint/issues/3228