nolintlint: reports false positive warnings about used nolint directive
Welcome
- [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
- [X] Yes, I've searched similar issues on GitHub and didn't find any.
- [X] Yes, I've included all information below (version, config, etc).
- [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)
Description of the problem
We have staticcheck and nonolint enabled.
Sometimes in our CI builds golangci-lint reports a false warnings from nonolint about unused nolint:staticcheck comments.
All warnings that I checked are wrong, the //nolint:staticcheck directives always silence existing deprecation warnings from staticcheck.
I was not able to reproduce this issue. In CI we reuse the golangci-lint and go cache when running checks on different branches. It can happen that the same cache was used previously with older go and/or golangci-lint versions.
I believe https://github.com/golangci/golangci-lint/issues/1940#issuecomment-1239477698 refers to the same issue.
Version of golangci-lint
$ golangci-lint --version
golangci-lint has version 1.49.0 built from cc2d97f3 on 2022-08-24T10:24:37Z
Configuration file
$ cat .golangci.yml
run:
concurrency: 2
deadline: 5m
linters:
disable-all: true
enable:
- depguard
- errcheck
- exportloopref
- goimports
- goprintffuncname
- ineffassign
- misspell
- nolintlint
- revive
- rowserrcheck
- staticcheck
- typecheck
- unconvert
- unused
- vet
issues:
exclude-use-default: false
Go environment
$ go version && go env
[/src/go/code/orders]$ go version && go env
go version go1.19.1 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN="/usr/local/bin"
GOCACHE="/var/cache/go-build"
GOENV="/tmp/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS="-tags=timetzdata -trimpath"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/var/cache/go-modcache"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
GOMOD="/src/go/go.mod"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1328424340=/tmp/go-build -gno-record-gcc-switches"
Verbose output of running
N/A
Code example or link to a public repository
Example case, output from golangci-lint:
[..]/itemdetails.go:36:32: directive `//nolint:staticcheck // Description field is deprecated` is unused for linter "staticcheck" (nolintlint)
return item.Description, nil //nolint:staticcheck // Description field is deprecated
The referenced Description field has a deprecation comment set:
// Deprecated: Do not use.
Description string `protobuf:"bytes,4,opt,name=description,proto3" json:"description,omitempty"`
Hello,
I change some elements related to facts (they are used by staticcheck) maybe the problem will be fixed with the next release (v1.50.0)
The issue happend again with golangci-lint 1.50.0.
bump
Happened with golangci-lint v1.54.2, too.
Have this issue also when dealing with switch/case: nolint:exhaustive` is unused for linter "exhaustive" (nolintlint) v1.55.2
If you are facing the same problem, the best way to contribute is to provide the following information:
golangci-lint --version
cat .golangci.yml
go version && go env
golangci-lint cache clean
golangci-lint run -v
And a code example or link to a public repository.
Here's a reliable repro:
#!/bin/sh -e
## setup module
rm -rf nolintrepro
mkdir nolintrepro && cd $_
## go.mod
cat > go.mod <<EOF
module example.com/nolintrepro
go 1.21
require github.com/jackc/pgx/v5 v5.5.3
require (
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
golang.org/x/crypto v0.17.0 // indirect
golang.org/x/text v0.14.0 // indirect
)
EOF
## main.go
cat > main.go <<EOF
package main
import (
"fmt"
"github.com/jackc/pgx/v5"
)
func main() {
var conn *pgx.Conn
fmt.Println(conn.PgConn().CheckConn() == nil)
fmt.Println(conn.PgConn().CheckConn() == nil) //nolint: staticcheck
}
EOF
go mod tidy
## .golangci.yml
cat > .golangci.yml <<EOF
linters:
disable-all: true
enable:
- staticcheck
- nolintlint
EOF
## Run
golangci-lint run
## Clean
cd ..
rm -rf nolintrepro
version and env
$ go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
$ golangci-lint --version
golangci-lint has version v1.56.2 built with go1.22.0 from (unknown, mod sum: "h1:dgQzlWHgNbCqJjuxRJhFEnHDVrrjuTGQHJ3RIZMpp/o=") on (unknown)
$ go version && go env
go version go1.22.0 darwin/arm64
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/me/Library/Caches/go-build'
GOENV='/Users/me/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/me/.local/share/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/me/.local/share/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/me/nolintrepro/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/yd/g6bkd_fd7qs40y6q_8w2fh2w0000gn/T/go-build3765820227=/tmp/go-build -gno-record-gcc-switches -fno-common'
$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/me/nolintrepro /Users/me /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 2 linters: [nolintlint staticcheck]
INFO [loader] Go packages loading at mode 575 (deps|name|types_sizes|compiled_files|exports_file|files|imports) took 523.977875ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 161µs
INFO [linters_context/goanalysis] analyzers took 2.663033296s with top 10 stages: buildir: 2.039481668s, fact_deprecated: 150.111285ms, nilness: 126.620753ms, fact_purity: 125.42288ms, SA5012: 115.692744ms, typedness: 103.558214ms, SA4006: 222µs, isgenerated: 130.292µs, SA4027: 99.542µs, SA9004: 88.75µs
INFO [runner] Issues before processing: 3, after processing: 1
INFO [runner] Processors filtering stat (out/in): path_shortener: 1/1, cgo: 3/3, filename_unadjuster: 3/3, uniq_by_line: 1/1, nolint: 1/3, severity-rules: 1/1, fixer: 1/1, skip_files: 3/3, autogenerated_exclude: 3/3, exclude: 3/3, max_from_linter: 1/1, skip_dirs: 3/3, identifier_marker: 3/3, diff: 1/1, max_same_issues: 1/1, source_code: 1/1, path_prefixer: 1/1, sort_results: 1/1, path_prettifier: 3/3, exclude-rules: 3/3, max_per_file_from_linter: 1/1
INFO [runner] processing took 519.497µs with stages: exclude-rules: 189.292µs, nolint: 94.251µs, autogenerated_exclude: 71.416µs, identifier_marker: 69.291µs, source_code: 52.833µs, path_prettifier: 31.583µs, skip_dirs: 4.959µs, uniq_by_line: 1.417µs, cgo: 1.041µs, max_same_issues: 749ns, path_shortener: 458ns, max_from_linter: 417ns, filename_unadjuster: 292ns, sort_results: 292ns, diff: 209ns, fixer: 208ns, max_per_file_from_linter: 208ns, skip_files: 167ns, severity-rules: 166ns, exclude: 166ns, path_prefixer: 82ns
INFO [runner] linters took 1.05964275s with stages: goanalysis_metalinter: 1.059068917s
main.go:11:14: SA1019: conn.PgConn().CheckConn is deprecated: CheckConn is deprecated in favor of Ping. CheckConn cannot detect all types of broken connections where the write would still appear to succeed. Prefer Ping unless on a high latency connection. (staticcheck)
fmt.Println(conn.PgConn().CheckConn() == nil)
^
INFO File cache stats: 1 entries of total size 213B
INFO Memory: 19 samples, avg is 131.1MB, max is 309.6MB
INFO Execution took 1.774828083s
This is the correct behavior—staticcheck is flagging the line without the nolint directive.
But if we modify the file (which I assume affects the cache), we get the incorrect behavior. I added a newline after var conn *pgx.Conn to make a change that should be equivalent from an AST perspective. Then ran golangci-lint run -v again:
INFO [config_reader] Config search paths: [./ /Users/me/nolintrepro /Users/me /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 2 linters: [nolintlint staticcheck]
INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|types_sizes|compiled_files|deps|name) took 531.612584ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 198.542µs
INFO [linters_context/goanalysis] analyzers took 8.812332ms with top 10 stages: buildir: 5.229666ms, SA5012: 532.211µs, fact_purity: 447.049µs, nilness: 416.708µs, fact_deprecated: 405.157µs, typedness: 365.661µs, SA4001: 78.208µs, SA1021: 77.834µs, SA6005: 70.25µs, SA4026: 65.584µs
INFO [runner] Processors filtering stat (out/in): exclude: 1/1, diff: 1/1, source_code: 1/1, fixer: 1/1, cgo: 1/1, filename_unadjuster: 1/1, skip_dirs: 1/1, autogenerated_exclude: 1/1, uniq_by_line: 1/1, severity-rules: 1/1, skip_files: 1/1, exclude-rules: 1/1, nolint: 1/1, max_same_issues: 1/1, max_from_linter: 1/1, path_shortener: 1/1, path_prettifier: 1/1, identifier_marker: 1/1, max_per_file_from_linter: 1/1, path_prefixer: 1/1, sort_results: 1/1
INFO [runner] processing took 224.418µs with stages: nolint: 57.251µs, autogenerated_exclude: 51.125µs, source_code: 27.209µs, path_prettifier: 25.876µs, identifier_marker: 25.125µs, exclude-rules: 25.042µs, skip_dirs: 7.542µs, uniq_by_line: 1.042µs, max_same_issues: 1.042µs, max_from_linter: 584ns, cgo: 541ns, path_shortener: 459ns, filename_unadjuster: 292ns, fixer: 208ns, skip_files: 208ns, exclude: 208ns, severity-rules: 207ns, sort_results: 166ns, max_per_file_from_linter: 125ns, diff: 125ns, path_prefixer: 41ns
INFO [runner] linters took 88.876792ms with stages: goanalysis_metalinter: 88.6125ms
main.go:13:48: directive `//nolint: staticcheck` is unused for linter "staticcheck" (nolintlint)
fmt.Println(conn.PgConn().CheckConn() == nil) //nolint: staticcheck
^
INFO File cache stats: 1 entries of total size 214B
INFO Memory: 10 samples, avg is 30.4MB, max is 51.4MB
INFO Execution took 819.347ms
Now it complains about the unused nolint directive, and staticcheck doesn't complain at all.
If you run golangci-lint cache clean, the correct behavior returns until the file is modified again.
Thank you for the repro.
As a workaround for staticcheck, I recommend using the following example instead of nolint directives:
issues:
exclude-rules:
- linters:
- staticcheck
text: "SA1019: conn.PgConn().CheckConn is deprecated: CheckConn is deprecated in favor of Ping"
Side note: a directive should not contain space after the ::
nolint: xxxnot validnolint:xxxvalid.
Seeing this in combination with ireturn too, as mentioned the error goes away for a time if you clean the cache with golangci-lint cache clean.
pkg/mything.go:9:78: directive `//nolint:ireturn // Returns one of multiple types which implement MyThinger.` is unused for linter "ireturn" (nolintlint)
func Testfunc() MyThinger { //nolint:ireturn // Returns one of multiple types which implement MyThinger.
~~I think this issue is the same as #4423, I will check my fix #4424 on that.~~ EDIT no it's not the same problem.
May be related to #3476
@stevenh If you are facing the same problem, the best way to contribute is to provide the following information:
golangci-lint --version
cat .golangci.yml
go version && go env
golangci-lint cache clean
golangci-lint run -v
And a code example or link to a public repository.