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

nonolint fix actual nolint:exhaustive #1940

Open
4 tasks done
Antonboom opened this issue Apr 28, 2021 · 11 comments
Open
4 tasks done

nonolint fix actual nolint:exhaustive #1940

Antonboom opened this issue Apr 28, 2021 · 11 comments
Labels
area: nolint Related to nolint directive and nolintlint bug Something isn't working dependencies Relates to an upstream dependency

Comments

@Antonboom
Copy link
Contributor

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).
  • Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)
Description of the problem

--fix remove actual nolint directive.

Version of golangci-lint
$ golangci-lint --version
golangci-lint has version 1.39.0 built from 9aea4ae on 2021-03-26T10:52:04Z
Config file
$ 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
$ 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
$ 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
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 Antonboom added the bug Something isn't working label Apr 28, 2021
@Antonboom
Copy link
Contributor Author

Antonboom commented Apr 28, 2021

And magic. Such code linted ok:

func adaptRPState(s fsmrp.StateName) pbRp.State { //nolint:nolintlint
	switch s {

@Antonboom
Copy link
Contributor Author

After another yet cache cleaning last working variant:

func adaptRPState(s fsmrp.StateName) pbRp.State { //nolint:nolintlint
	switch s { //nolint:exhaustive

@ldez
Copy link
Member

ldez commented Apr 28, 2021

Hello,

I created a minimal example:

main.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
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 ldez added the feedback required Requires additional feedback label Apr 28, 2021
@ldez
Copy link
Member

ldez commented Apr 29, 2021

@Antonboom any feedback?

@Antonboom
Copy link
Contributor Author

Antonboom commented Apr 30, 2021

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
Copy link
Contributor Author

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

@ldez ldez added dependencies Relates to an upstream dependency and removed feedback required Requires additional feedback labels Apr 30, 2021
@ldez
Copy link
Member

ldez commented Apr 30, 2021

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

@nishanths
Copy link
Contributor

nishanths commented Jul 14, 2021

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

@powerman
Copy link

powerman commented Aug 6, 2022

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

@ian-h-chamberlain
Copy link

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

This comment was marked as off-topic.

justinsb added a commit to justinsb/kustomize that referenced this issue Mar 20, 2023
The lint task was failing at head, due to a nolint:exhaustive error
directive that golangci nolintlint believes is unused.

Issue seems to be golangci/golangci-lint#3228
and seems to be a bug in golang-ci / nolintlint, using the workaround
proposed in golangci/golangci-lint#1940
which is to clear the cache between runs.
@ldez ldez added the area: nolint Related to nolint directive and nolintlint label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nolint Related to nolint directive and nolintlint bug Something isn't working dependencies Relates to an upstream dependency
Projects
None yet
Development

No branches or pull requests

6 participants